Skip to content

Conversation

@United600
Copy link
Collaborator

  • Adds a PowerShell script that finds UFO font packages, invokes FontForge to generate the custom fonts, and optionally moves them to the application assets folder if the -Publish flag is used
  • Adds a FontForge script that batch-converts UFO packages to TrueType fonts

IMO, an Azure DevOps or GitHub Actions workflow would still be the best approach.

@United600 United600 requested a review from Copilot January 28, 2026 18:34
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Jan 28, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds local tooling to generate TrueType icon fonts from UFO packages using FontForge, with an option to publish the generated fonts into the app assets.

Changes:

  • Adds scripts/Generate-Fonts.ps1 to discover .ufo packages and invoke FontForge for batch conversion, with optional publishing.
  • Adds assets/fonts/fontforge-ufo-to-ttf.pe as the FontForge batch conversion script.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
scripts/Generate-Fonts.ps1 PowerShell wrapper to locate UFO packages, run FontForge conversion, and optionally move generated .ttf files into app assets.
assets/fonts/fontforge-ufo-to-ttf.pe FontForge script to convert provided UFO paths into .ttf outputs.

Comment on lines +51 to +59
if ($Publish) {
try {
$ttfFiles = @(Get-ChildItem -Path $fontsPath -Filter '*.ttf' -File -ErrorAction SilentlyContinue | Select-Object -ExpandProperty FullName)

if ($ttfFiles.Count -eq 0) {
Write-Error "No .ttf files found in the folder: $fontsPath" -Category ObjectNotFound
exit 1
}

Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script invokes FontForge but never checks whether the external process succeeded. As written, it will exit 0 even if fontforge fails (and with -Publish omitted, it also doesn’t verify that any .ttf outputs were produced). Capture and validate the FontForge exit code (e.g., $LASTEXITCODE) and/or check for expected output files before returning success.

Suggested change
if ($Publish) {
try {
$ttfFiles = @(Get-ChildItem -Path $fontsPath -Filter '*.ttf' -File -ErrorAction SilentlyContinue | Select-Object -ExpandProperty FullName)
if ($ttfFiles.Count -eq 0) {
Write-Error "No .ttf files found in the folder: $fontsPath" -Category ObjectNotFound
exit 1
}
$fontForgeExitCode = $LASTEXITCODE
if ($fontForgeExitCode -ne 0) {
Write-Error "FontForge failed with exit code $fontForgeExitCode." -Category InvalidOperation
exit $fontForgeExitCode
}
$ttfFiles = @(Get-ChildItem -Path $fontsPath -Filter '*.ttf' -File -ErrorAction SilentlyContinue | Select-Object -ExpandProperty FullName)
if ($ttfFiles.Count -eq 0) {
Write-Error "No .ttf files found in the folder: $fontsPath" -Category ObjectNotFound
exit 1
}
if ($Publish) {
try {

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,7 @@
#!/usr/local/bin/fontforge
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shebang hardcodes /usr/local/bin/fontforge, which is not portable (and it’s unnecessary when this file is executed via fontforge -script). Consider removing the shebang entirely, or switching to an env-based shebang if you intend the script to be directly executable on Unix-like systems.

Suggested change
#!/usr/local/bin/fontforge
#!/usr/bin/env fontforge

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was taken straight from the FontForge scripting documentation, it does say "(or wherever fontforge happens to reside on your system)".

Either way, it's inconsequential for us. I think...

@huynhsontung
Copy link
Owner

huynhsontung commented Feb 1, 2026

Since this is something the app consumes, it would make more sense to run it as a build step, probably a pre-build target.

More details: MSBuild incremental builds for new or stale targets

@United600
Copy link
Collaborator Author

Since this is something the app consumes, it would make more sense to run it as a build step, probably a pre-build target.

That was one of the goals, but the main issue is that it relies on having another application installed (unfortunately no dotnet-tools.json support). For sure, the store build should have it as a step (same for the Lottie gen).

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 4, 2026
@huynhsontung
Copy link
Owner

I think I can implement that once we move to .NET 10 and SDK style project file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants